Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.gitignore:
- Around line 17-18: Remove the overly-broad ignore patterns '*.md' and '*.txt'
from .gitignore and replace them with specific patterns or explicit exclusions:
delete the two lines, add narrow ignores for only generated or local files
(e.g., build/*.md, reports/*.txt) or add explicit allow rules for important docs
(e.g., !README.md, !CONTRIBUTING.md, !CHANGELOG.md) so documentation remains
tracked; if those patterns were intended only for personal temp files, move them
to your local .git/info/exclude instead.

In `@src/controllers/analytics.controller.ts`:
- Around line 82-86: The storageUsedPercentage calculation divides by
user.storage_quota without checks; update the object creation (the
storageUsedPercentage field that uses overview.storage.totalSize /
user.storage_quota) to first verify that user.storage_quota is a positive number
(e.g., Number(user.storage_quota) > 0) and only then compute
((overview.storage.totalSize / user.storage_quota) * 100).toFixed(2); if the
quota is zero/invalid, return a safe fallback (e.g., "0.00" or null) to avoid
NaN/Infinity.
- Around line 42-55: After parsing startDateStr and endDateStr into startDate
and endDate, add a validation that startDate.getTime() <= endDate.getTime(); if
not, return the same res.FailureResponse pattern with a 400 status and a clear
message (e.g., "Invalid date range: startDate must be before or equal to
endDate") before calling analyticsService.getAnalyticsByDateRange(user.id,
startDate, endDate).

In `@src/controllers/file.controller.ts`:
- Around line 36-43: The calls to addToAnalyticsQueue are asynchronous and
currently fire-and-forget, which can produce unhandled promise rejections if
Redis/enqueue fails; modify each call to addToAnalyticsQueue (including the ones
used for folder creation, updates, and deletions) to handle failures by either
awaiting the promise inside an async try/catch or by appending a .catch(...)
that logs the error (using the existing logger/processLogger) so rejections are
swallowed and surfaced; ensure every invocation of addToAnalyticsQueue in
file.controller.ts is updated to handle errors consistently.

In `@src/controllers/upload.controller.ts`:
- Around line 38-52: The fire-and-forget call to addToAnalyticsQueue inside
files.forEach can produce unhandled rejections; protect it by attaching a local
catch handler to the promise returned by addToAnalyticsQueue (or wrap the call
in a try/catch if you convert the iterator to an async loop) so transient queue
failures are swallowed or logged. Specifically, in the upload controller where
files.forEach is used and addToAnalyticsQueue({ userId: user.id, eventType:
AnalyticsEventType.FILE_UPLOADED, ... }) is invoked, ensure you call the
function and then chain .catch(err => /* log or noop */) so any rejection is
handled locally and does not create an unhandled rejection.

In `@src/core/analytics-queue.ts`:
- Around line 50-53: The Queue/Worker/QueueEvents instances (analyticsQueue and
associated Worker/QueueEvents created around lines 60-100) are left open on
import; add and export a shutdownAnalyticsQueue function that gracefully closes/
disconnects analyticsQueue, its Worker instance, and QueueEvents by calling
their close()/disconnect() or waitUntilReady()/close equivalents and awaiting
them, ensure it handles in-flight jobs (drain/stop worker then close) and is
idempotent; then call shutdownAnalyticsQueue from the process shutdown hook
(SIGINT/SIGTERM) in your app entry to ensure Redis handles are released and jobs
drained on redeploy.

In `@src/index.ts`:
- Around line 41-43: The protocol log is derived only from
config.HTTP2.SSL.ENABLED which can be true even if TLS cert/key are missing;
change the logic that sets protocol (the variable named protocol near
displayHost/port) to check both the SSL enabled flag and that the configured
certificate and key are actually present (e.g., config.HTTP2.SSL.CERT and
config.HTTP2.SSL.KEY or whatever TLS cert/key fields your config uses) and
ideally readable, then use "https" only when both enabled and cert+key are
available, otherwise fall back to "http".

In `@src/models/Notification.model.ts`:
- Around line 4-7: The DB enum backing NotificationType needs a migration to add
the FILE_SHARE_REVOKED value so runtime writes from code using
NotificationType.FILE_SHARE_REVOKED (referenced in
src/models/Notification.model.ts and used in src/controllers/file.controller.ts)
do not fail; create a new Sequelize migration that alters the PostgreSQL enum
(or creates a new enum and replaces the column) to include 'file_share_revoked'
(using ALTER TYPE ... ADD VALUE 'file_share_revoked' or the safe create-new-enum
+ ALTER COLUMN ... USING ... pattern for older PG versions), run the migration
to update the DB, and then ensure the codebase uses the same enum literal string
as NotificationType.FILE_SHARE_REVOKED.

In `@src/repository/analytics.repository.ts`:
- Around line 146-148: The updates object currently does a read-modify-write
(analytics.uploads_today + 1) which will undercount under concurrency; change
the logic to perform an atomic increment in the database instead of computing
uploads_today in-memory — e.g., use your ORM/connector's atomic increment/update
operation for the uploads_today column (reference variables/fields: analytics,
uploads_today, updates, and storage.totalFiles) and apply the same change for
the other occurrence at the block referencing lines 163-164 so concurrent
workers increment the counter atomically rather than overwriting each other.

In `@src/scripts/migrate-storage.ts`:
- Around line 110-112: migrateFile is intended to stream but currently downloads
whole objects into buffers and re-uploads them, causing OOM for large files;
change migrateFile to open a readable stream from the source (use the SDK method
that returns a stream/streamable response) and pipe or stream that data directly
into the destination upload (use the SDK uploadStream/multipart upload API or a
Writeable stream) instead of buffering the full content, and apply the same
streaming approach to the other places mentioned (the code around the blocks
referenced at 162-167 and 176-182) so all downloads -> uploads are performed via
streaming/piping rather than Buffer allocations.
- Line 113: The code currently flattens object keys with const fileName =
key.split('/').pop() || key, causing different source paths to collide; instead
preserve the relative path (e.g., compute relativeKey by stripping the source
prefix or base folder from key) and use that relativeKey when constructing the
destination object name so directory structure is retained; replace any usage of
fileName in the migration logic (including the blocks around where fileName is
set and the similar logic at the other affected spots) with this relativeKey (or
properly normalized path) so keys remain unique and overwrite collisions are
avoided.
- Around line 287-292: The current validation in main() only checks secondary S3
env vars and lets missing primary S3 credentials surface later; update the
startup validation to also fail fast for primary S3 by checking
PRIMARY_S3_TOKEN_ID, PRIMARY_S3_SECRET_KEY, PRIMARY_S3_ENDPOINT and
PRIMARY_S3_BUCKET_NAME alongside the secondary vars (or factor into a helper
like validateS3EnvVars used for both primary and secondary) and throw a clear
Error when any required primary variable is missing so the script exits
immediately instead of running partial transfers.
- Line 39: The default parameter for migrateFolder erroneously uses
FolderNameEnum.IMAGES instead of mirroring the provided sourceFolder; change the
signature so targetFolder is optional (remove the FolderNameEnum.IMAGES default)
and inside migrateFolder set targetFolder = targetFolder ?? sourceFolder (or
assign to a local variable like const destination = targetFolder ??
sourceFolder) so when callers omit targetFolder it defaults to the sourceFolder;
update references in the function to use that resolved destination and keep the
FolderNameEnum type for targetFolder.

In `@src/services/analytics.service.ts`:
- Around line 24-31: The check in the file upload branch uses a truthy test on
metadata.fileSize which drops valid zero-byte uploads; change the condition in
the case 'file_uploaded' to explicitly test for presence (e.g. metadata.fileSize
!== undefined && metadata.fileType != null) before calling
analyticsRepository.recordFileUpload(userId, metadata.fileSize,
metadata.fileType) so fileSize === 0 is accepted but missing values are still
rejected.

In `@src/services/lock.service.ts`:
- Around line 20-22: Replace the current simple set/delete lock logic with
ownership tokens and an atomic compare-and-delete: when acquiring a lock (where
client.set(fullKey, Date.now().toString(), 'EX', ttl, 'NX') is used) write a
strong random token (e.g., UUID) as the key value and return that token to the
caller instead of Date.now(); when releasing (the places that currently call
client.del(fullKey) in the unlock/finally blocks referenced around lines 37-43
and 79-80) use an atomic Lua script that checks the key's value equals the token
and only then deletes it (EVAL with: if redis.call("GET", KEYS[1]) == ARGV[1]
then return redis.call("DEL", KEYS[1]) else return 0 end); update the lock API
to accept/propagate the token so unlock can supply it and ensure all unlock
paths use the compare-and-delete script rather than blind DEL.

In `@src/utils/cors-options.ts`:
- Around line 11-13: The CORS allowlist currently hardcodes a private LAN origin
("http://192.168.0.8:5173") which should not be used in production; update the
code that defines the allowedOrigins/ corsOptions array to remove hardcoded
private IPs and instead load dev-only origins from an environment variable
(e.g., process.env.DEV_CORS_ORIGINS split by comma) or only add local/dev
origins when NODE_ENV !== 'production'; ensure credentialed CORS (with
credentials: true) only uses trusted production origins from a secure env var
like PROCESS_CORS_ORIGINS and guard any dev origins behind a runtime check so
private LAN addresses are never included in production allowlists.

---

Nitpick comments:
In @.github/workflows/ci-cd.yml:
- Around line 35-45: The workflow currently sets provenance: false in the
docker/build-push-action@v6 step which disables SLSA provenance; change the
provenance setting to true (or make it conditional via an env flag) so
provenance attestation is emitted when the target registry supports it, or if
you must keep it false add a clear comment and README note explaining why;
update the docker/build-push-action@v6 step (the block containing context, push,
platforms, provenance, tags, cache-from, cache-to) to either set provenance:
true or read an env variable like PROVENANCE_ENABLED to toggle it and document
the rationale for any deliberate disablement.

In `@src/repository/analytics.repository.ts`:
- Around line 144-145: The code is performing full user file-table scans by
calling calculateUserStorage(userId) on every upload/delete/folder event which
will not scale; instead implement incremental storage bookkeeping: add a
persistent user_storage (or storage_bytes) column and update it by applying the
delta (e.g., +file.size on upload, -file.size on delete, adjust on
move/overwrite) inside the event handlers rather than recomputing via
calculateUserStorage each time; keep calculateUserStorage only as a fallback
reconciliation method and ensure event handlers (the upload/delete/folder
handlers that currently call calculateUserStorage) compute and persist deltas
atomically (transactionally) so concurrent events are safe.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/repository/file.repository.ts`:
- Around line 525-542: Update getAllChildFileIds to traverse only trashed
descendants owned by the relevant user, filtering each recursive level by
deleted_at IS NOT NULL so individually restored files are excluded from
emptyTrash deletion. Prefer a loop-safe recursive CTE or equivalent visited-node
protection while preserving collection of all eligible descendant IDs.
- Around line 544-595: Update emptyTrash to hard-delete the collected database
rows before touching S3, using the repository’s transaction pattern where
applicable. Chunk s3Keys into batches of at most 1,000 before calling
s3Service.deleteFiles, and handle failed S3 batches through the existing
reconciliation mechanism so the operation does not leave dangling trash rows or
silently omit objects.